Skip to content

Кулешова Ирина#53

Open
trtz wants to merge 6 commits intourfu-2016:masterfrom
trtz:master
Open

Кулешова Ирина#53
trtz wants to merge 6 commits intourfu-2016:masterfrom
trtz:master

Conversation

@trtz
Copy link
Copy Markdown

@trtz trtz commented Nov 10, 2016

No description provided.

@honest-hrundel
Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link
Copy Markdown

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link
Copy Markdown

🍏 Пройдено тестов 9 из 9

off: function (event, context) {
console.info(event, context);
var eventsToOff = Object.keys(this._events).filter(
function (key) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давай переименуем key

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и еще эту строку можно к предыдущей приписать, так обычно пишут

function (key) {
return key === event ||
(key.length > event.length &&
key.substr(0, event.length + 1) === event + '.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вроде из условий

(key.length > event.length && key.substr(0, event.length + 1) === event + '.')

достаточно написать последнее

key.substr(0, event.length + 1) === event + '.');
}
);
for (var i = 0; i < eventsToOff.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему бы не использовать forEach?

emit: function (event) {
console.info(event);
var events = getAllEvents(event).reverse();
for (var i = 0; i < events.length; i++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и тут вроде forEach хорошо впишется, тогда не придется создавать новую переменную строчкой ниже



function getAllEvents(event) {
var splitted = event.split('.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Название не отражает, что лежит в переменной

*/
emit: function (event) {
console.info(event);
var events = getAllEvents(event).reverse();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если ты сплитишь event, а потом реверсишь, то почему бы сразу не пойти с конца?

);
for (var i = 0; i < eventsToOff.length; i++) {
this._events[eventsToOff[i]] = this._events[eventsToOff[i]].filter(
function (item) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут item переименовать надо

@bazhenova
Copy link
Copy Markdown

🍅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants